Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING] Clone resources when writing in and reading from the Memory #448

Merged
merged 11 commits into from
Dec 14, 2022

Conversation

flovogt
Copy link
Member

@flovogt flovogt commented Nov 29, 2022

Clone resources when writing them in the memory.

JIRA: CPOUI5FOUNDATION-567

Follow-Up of #259.

@coveralls
Copy link

coveralls commented Nov 29, 2022

Coverage Status

Coverage increased (+2.0%) to 88.644% when pulling 0678d02 on memadapter-clone-resource-write into 62482f5 on main.

@RandomByte
Copy link
Member

LGTM

@flovogt
Copy link
Member Author

flovogt commented Dec 2, 2022

  • Cherry-pick "read" PR in this one here and close the other one
  • Enhance integration test "test/lib/resources.js" to call setString, setPath to the written resource and check that original resource is not changed for both existing adapters
  • add unit test for _write method in the AbstractAdapter to catch Unable to write resource associated with project

@flovogt flovogt changed the title [BREAKING] Clone resources when writing in the Memory [BREAKING] Clone resources when writing in and reading from the Memory Dec 12, 2022
@flovogt flovogt force-pushed the memadapter-clone-resource-write branch from add6c92 to 5c45184 Compare December 12, 2022 12:20
@flovogt flovogt merged commit 3454bc1 into main Dec 14, 2022
@flovogt flovogt deleted the memadapter-clone-resource-write branch December 14, 2022 14:39
RandomByte added a commit that referenced this pull request Dec 22, 2022
When cloning a Resource, the new Resource instance should not be
associated with any project. This allows for use cases where a resource
is copied from one project to another.

The problematic behavior was introduced with
#448 and motivated by the need of the
memory adapter to clone resources while keeping the project association.

However, since memory adapters are always assigned a project, they can
easily just assign that project to the cloned resource. This has been
implemented in this PR.
RandomByte added a commit that referenced this pull request Dec 22, 2022
When cloning a Resource, the new Resource instance should not be
associated with any project. This allows for use cases where a resource
is copied from one project to another.

The problematic behavior was introduced with
#448 and motivated by the need of the
memory adapter to clone resources while keeping the project association.

However, since memory adapters are always assigned a project, they can
easily just assign that project to the cloned resource. This has been
implemented in this PR.
RandomByte added a commit that referenced this pull request Dec 22, 2022
When cloning a Resource, the new Resource instance should not be
associated with any project. This allows for use cases where a resource
is copied from one project to another.

The problematic behavior was introduced with
#448 and motivated by the need of the
memory adapter to clone resources while keeping the project association.

However, since memory adapters are always assigned a project, they can
easily just assign that project to the cloned resource. This has been
implemented in this PR.
matz3 added a commit that referenced this pull request Jan 4, 2023
Some tests didn't use the corresponding adapter, which caused
race-conditions as both adapter tests used the FileSystem adapter.

It is now ensured that all tests use the right adapter.

Follow-up of #448
matz3 added a commit that referenced this pull request Jan 9, 2023
Some tests didn't use the corresponding adapter, which caused
race-conditions as both adapter tests used the FileSystem adapter.

It is now ensured that all tests use the right adapter.

Follow-up of #448
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants